Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi lang support #26

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ettoreleandrotognoli
Copy link

Hello,
I'm trying to resolve this issue #25

I was able to generate the example, and it is working, but even from the main branch I was not able to run the tests, at least I can't figure out how to do it.

@willcrichton
Copy link
Collaborator

Thanks for the contribution! I would like mdbook-quiz to support syntax highlighting for multiple languages. However, the issue with the current PR is that it increases the bundle size by 50%, from ~2MB to ~3MB. I specifically avoided doing this:

import hljs from "highlight.js";

Because that includes by default every language, which is a lot of code. I think the best way to support multiple languages without bloating the bundle would be the following:

  1. User provides a list of languages they want to include in the bundle as an environment variable. For example:

    MDBOOK_QUIZ_LANGS="rust,cpp" cargo install mdbook-quiz
    
  2. The snippet.tsx script imports a fake script like:

    import "highlightjs-languages";
  3. An esbuild plugin for the quiz package provides a dynamically-generated source for this fake file based on the compile-time environment variable:

    build.onResolve({filter: /^highlightjs-languages$/ }, args => ({ path: args.path, namespace: "highlightjs-languages" })
    build.onLoad({filter: /.*/, namespace: "highlightjs-languages"}, () => { 
      let contents = buildScript(process.env.MDBOOK_QUIZ_LANGS);
      return {contents};
    });

I would accept a version of this PR that adopts this or a comparable strategy.

@ettoreleandrotognoli
Copy link
Author

Hello @willcrichton , thanks for the feedback, I'll work on that ^^

I have some questions:

As mdbook supports some languages by default, would be better to support the same languages by default too?
https://rust-lang.github.io/mdBook/format/theme/syntax-highlighting.html?highlight=syntax%20he#syntax-highlighting

As mdbook already uses Highlight.js do you think there is a way to use the same instance instead of importing it again in this library?

@willcrichton
Copy link
Collaborator

Yes, it would be fine to use the mdBook highlight.js library. However, it's important that the quiz package does not rely on the existence of window.highlightjs, since that package is used in other places beyond just mdbook-quiz. I am fine with a solution that does not bundle highlight.js into quiz, and then has quiz-embed carefully provide the global highlight.js to quiz.

@willcrichton willcrichton force-pushed the main branch 6 times, most recently from bc8d578 to fa71479 Compare September 21, 2023 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants